-
-
Notifications
You must be signed in to change notification settings - Fork 225
fix: Ensure structured logs from an SDK integration has sentry.origin #4566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Ensure structured logs from an SDK integration has sentry.origin #4566
Conversation
@sentry review |
bugbot review |
This comment has been minimized.
This comment has been minimized.
🚨 Bugbot couldn't runSomething went wrong. Try again by commenting "Cursor review" or "bugbot run", or contact support (requestId: serverGenReqId_8ec61e90-31f9-40d2-9718-56e6535d466d). |
bugbot review |
@sentry review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4566 +/- ##
=======================================
Coverage 73.46% 73.47%
=======================================
Files 482 482
Lines 17678 17682 +4
Branches 3493 3493
=======================================
+ Hits 12988 12992 +4
Misses 3800 3800
Partials 890 890 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100 % convinced of the origin names that we use right now, but also don't have a concrete suggestion in store ... if you don't mind, I'll let my mind wander a bit and will get back to you soon.
Also, feedback from other reviewers appreciated. @jamescrosswell what do you think?
From memory, we started setting the
|
Whoops, forgot to mention that we will be consolidating the category for |
The Structured Logs spec was updated. See #4618 for more detail: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for aligning the value of "sentry.origin".
One final feedback to fix the CHANGELOG.md
.
Then we could release this in 5.16.1
.
Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: James Crosswell <[email protected]>
163dff3
to
6608542
Compare
Fixes #4560, fixes #4618
sentry.origin
attribute attached #4560sentry.origin
for Structured Logs according to the documentation #4618Problem
According to our SDK specification, we have to set
sentry.origin
whenever a log is sent from an integration.Solution
sentry-dotnet
has a few integrations that send out logs. This PR affects the following integrations:Sentry.Extensions.Logging
Sentry.Serilog
And affects other integrations that references
Sentry.Extensions.Logging
, or references it transitively.Comparison
Before, logs from

Sentry.Serilog
didn't havesentry.origin
:After, logs from

Sentry.Serilog
hassentry.origin
populated: